Fix 2D morphology coloring to use per-compartment values#60
Conversation
Previously, 2D morphology plotting applied a single color per section, effectively using only the first compartment’s mapped value. This caused incorrect visualization when values differed across compartments. In _plot_morphology2D, colors are now computed and applied per compartment. - For show_diameter=False: each compartment is drawn as an individual segment with its own color. - For show_diameter=True: each compartment is drawn as its own polygon patch with compartment-specific diameters and color. - Scalar-like values are still supported by repeating one value across the section to preserve intentional single-color behavior. - 3D plotting logic remains unchanged. Added a regression test to verify per-compartment color differentiation. All morphology plotting tests pass.
|
Hi @Akshat1000Sharma Thank you for the PR, and apologies for having taken so long to reply. I did a first quick review, and I think it looks good 👍
|
- Replaced broad `except Exception` with more specific exception handling - Inlined `_section_colors` function into `_plot_morphology2D` - Preserved existing behavior for both scalar and per-compartment values
|
Thanks @mstimberg for your time and valuable feedback. I have incorporated the suggested changes:
I have also merged the latest master into this branch. |
|
Hi @mstimberg, would you like me to open a separate issue and submit another PR to address the failing test cases? I have reviewed the code and believe I can work on fixing them. |
|
Hi @Akshat1000Sharma, in principle I'd be happy for you to open a PR, but @ulekarsiddhant0-boop also had started working on it (see #68) – not sure whether he's still interested in opening a PR? |
|
Hi @Akshat1000Sharma, could you please fix the merge conflict with most recent master? The test suite on master shouldn't be failing anymore. |
|
Thank you for your message @mstimberg. I have updated my branch with most recent master branch. Please let me know if anything else is needed. |
|
Thanks, the PR itself looks good, but it seems as if the merge was not completely correct – the "FIXME" lines that were removed by #56 shouldn't be necessary anymore when using |
…e autoscaling workaround This commit fixes an issue introduced during the merge with upstream where `axes.add_artist()` was mistakenly used instead of `axes.add_patch()` in `_plot_morphology2D`. Using `add_artist()` prevented matplotlib from including the polygon in axis autoscaling, which required an additional invisible plotting workaround (`axes.plot(..., alpha=0.)`). This workaround is no longer necessary. Changes: - Replaced `axes.add_artist(patch)` with `axes.add_patch(patch)` - Removed obsolete FIXME comment and autoscaling workaround line The fix restores correct autoscaling behavior while preserving the per-compartment coloring logic introduced in this branch.
|
Hi @mstimberg, thanks for pointing that out. |
Remove the defensive ndim == 1 check on compartment_colors, which is unreachable since voltage_colormap always receives an array and returns a 2D (N, 4) RGBA result.
|
Hi @mstimberg, |
|
Hi Marcel,
Please accept my apologies for the late response—I have been caught up with
my first-year semester exams and college coursework.
I wanted to let you know that I've decided to focus my GSoC efforts this
year on the Apache Fury project. I found the low-level serialization
challenges there to be a great fit for my current interests.
I am very grateful for the guidance you and the Brian2 team provided,
especially during my work on the NumPy 2.0 migration. It was a great
learning experience, and I hope to cross paths with the INCF community
again in the future.
Best of luck with the Brian2 projects this summer!
Best regards,
Siddhant
…On Tue, Mar 31, 2026, 5:14 PM Marcel Stimberg ***@***.***> wrote:
Merged #60 <#60> into
master.
—
Reply to this email directly, view it on GitHub
<#60?email_source=notifications&email_token=BYISWRUC7QSRBZ7SSUICTG34TOVTHA5CNFSNUABQM5UWIORPF5TWS5BNNB2WEL2JONZXKZKFOZSW45CON52GSZTJMNQXI2LPNYXTENBQG4YTSNBZGIYDFJTSMVQXG33OU5WWK3TUNFXW5JLFOZSW45FMMZXW65DFOJPWG3DJMNVQ#event-24071949202>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BYISWRWXYJJ63VNF7RO2ICD4TOVTHAVCNFSM6AAAAACWDCUUZKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMRUGA3TCOJUHEZDAMQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
3 participants
Fixed #54
Previously, 2D morphology plotting applied a single color per section, effectively using only the first compartment’s mapped value. This caused incorrect visualization when values differed across compartments.
In _plot_morphology2D, colors are now computed and applied per compartment.
Added a regression test to verify per-compartment color differentiation.